Skip to content

refactor(woocommerce): replace inline SDK with automattic/woocommerce composer dependency (#264)#267

Open
Avatarsia wants to merge 30 commits intoOpenXE-org:masterfrom
Avatarsia:refactor/woocommerce-composer-sdk
Open

refactor(woocommerce): replace inline SDK with automattic/woocommerce composer dependency (#264)#267
Avatarsia wants to merge 30 commits intoOpenXE-org:masterfrom
Avatarsia:refactor/woocommerce-composer-sdk

Conversation

@Avatarsia
Copy link
Copy Markdown
Contributor

@Avatarsia Avatarsia commented Apr 21, 2026

TL;DR

Ersetzt die eingefrorene 2019er-Inline-Kopie (1374 Zeilen) des WooCommerce-PHP-SDK durch
die offizielle Composer-Dependency automattic/woocommerce:^3.1. Ein schmaler
ClientWrapper hält die in #265 genutzten Accessors (getLastResponse(), case-
insensitives getHeader()) verfügbar. Upstream-Patches — Security-Fixes,
PHP-Kompatibilität — fließen künftig per composer update ein.

⚠️ Wichtig beim Merge: Nach dem Git-Merge muss einmalig
composer install --no-dev auf dem Server ausgeführt werden, damit die neuen
vendor/automattic/-Dateien und der Autoloader-Index auf den Merge-Commit-Stand
synchronisiert werden.

📌 Stacked auf PR #265 (Pagination-Fix). Merge erst nach #265, dann Rebase
dieses Branches auf aktuellen master, dann Merge.

Closes #264.


Detaillierte Analyse (LLM-generiert, Claude Opus 4.7)

Was geändert wurde

Hauptverschiebung: 1374 Zeilen Inline-SDK aus www/pages/shopimporter_woocommerce.php
entfernt, ersetzt durch automattic/woocommerce als Composer-Dependency.

Neue Datei: classes/Components/WooCommerce/ClientWrapper.php (Namespace
Xentral\Components\WooCommerce) hält vier Aufgaben wach:

  1. getLastResponse() auf dem Client — upstream-SDK exponiert das nicht.
  2. Case-insensitiver getHeader($name) auf der Response-Wrapper-Klasse —
    upstream liefert nur getHeaders() als Array.
  3. PSR-3-Logger-Integration — HTTP ≥ 400 als ->warning(), Exception als
    ->error(), Rethrow unverändert (damit Call-Sites ihren try/catch-Kontrakt
    behalten).
  4. ssl_ignore-Mapping auf verify_ssl=false der Upstream-Options.

Die Businesslogik-Aufrufe ($this->client->get/post/…,
$this->client->getLastResponse()->getHeader(…)) bleiben unverändert — der Wrapper
hält den Kontrakt zu allen Pagination- und Batch-Stock-Logiken aus #265/#266.

Commits (chronologisch auf dem Branch)

  • 00707afecomposer require automattic/woocommerce
  • e433b5a7ClientWrapper (207 Zeilen) angelegt
  • 1a611230 — Inline-SDK entfernt, Namespace-Swap
  • af965010 — Logger-Integration im ClientWrapper (Review-Fix Medium)
  • ad804690, 2cfc64c0 — composer install metadata regenerate (Review-Fix Low)
  • f26fb82b.gitattributes: vendor/** von git diff --check-Whitespace
    ausnehmen (Review-Fix Low)

Test-Ergebnisse

End-to-End gegen Docker-WC-10.7 + WP 6.9.4 + PHP 8.3 auf Testinstanz — alle 7 PASS:

# Szenario Ergebnis
P1 Leere WC-DB, Count=0 PASS
P2 30 Orders Pagination PASS
P3 Same-second Order-Bucket (2 Orders, identisches date_created_gmt) PASS
P4 Idempotenz (zweiter Run) PASS
B1 50 Produkte Stock-Sync (Batch) PASS
B2 Ghost-SKU in Batch (19 OK, 1 Error-Log, kein Abbruch) PASS
L1 Bad Credentials → HTTP 401 + Logger-Eintrag PASS

Autoloader-Sanity verifiziert: Automattic\WooCommerce\Client und
Xentral\Components\WooCommerce\ClientWrapper beide via Composer-PSR-4 auflösbar.

Logger-Integration verifiziert:

LOGGER_WARNINGS: WooCommerce GET orders returned HTTP 401
LOGGER_ERRORS:   WooCommerce GET orders failed: Invalid signature...

Known Notes

  • Composer-Metadaten (vendor/composer/installed.php) referenzieren auf dem
    Branch technisch immer nur den vorletzten Commit — das Merge-composer install
    regeneriert die Datei auf den Merge-Commit. Selbstreferenz im Commit ist
    mathematisch nicht möglich.
  • vendor/** ist via .gitattributes von git diff --check-Whitespace-Prüfungen
    ausgenommen (Upstream-Code, unter unserer Kontrolle nicht).

Abgrenzung zu parallelen PRs/Issues

Rollback

Revert in toto. Das Inline-SDK lebt in der Git-Historie weiter und kommt durch die
Reverts automatisch wieder rein. Einziger Nacharbeits-Step nach Rollback:
composer install --no-dev erneut ausführen, um die alte vendor/-Struktur zu
synchronisieren.

Avatarsia pushed a commit to Avatarsia/OpenXE that referenced this pull request Apr 21, 2026
Adds the three WooCommerce PRs that are currently open against
openxe-org/openxe to the nightly production manifest:

- fix/woocommerce-order-import-pagination (PR OpenXE-org#265) — foundation
- feature/woocommerce-batch-stock-sync     (PR OpenXE-org#266) — independent
- refactor/woocommerce-composer-sdk         (PR OpenXE-org#267) — stacked on OpenXE-org#265,
  listed after it so the sequential merge in Pass 2 picks the dependency
  up first and the composer refactor applies on top cleanly.

Grouped under a dedicated header block so the intent stays visible
once the PRs merge upstream and can be removed from the manifest.
Avatarsia and others added 17 commits April 21, 2026 13:16
Adds getHeaders() / getHeader() accessors to the inline WCResponse class
and captures HTTP response headers case-insensitively via CURLOPT_HEADERFUNCTION.
Required foundation for pagination handling (Issue OpenXE-org#262).
Exposes the underlying WCResponse of the most recent request so callers
can read response headers (X-WP-Total, X-WP-TotalPages) without changing
the existing JSON-body return contract. Follow-up to 291197d, required
by the pagination work in issue OpenXE-org#262.
Reads felder.letzter_import_timestamp from shopexport.einstellungen_json
with a 30-day fallback for first runs, and adds a persistLastImportTimestamp()
helper that does a read-modify-write via DatabaseService named params.
Infrastructure for the pagination loop in issue OpenXE-org#262; not yet called here.
Replaces the fake greater-than-id filter (800 hardcoded IDs) with the
WC v3 after=<iso-8601> parameter and walks X-WP-TotalPages up to
MAX_PAGES_PER_RUN=5 pages per run (500 orders). Persists a progress
timestamp via persistLastImportTimestamp() after each processed order
so aborted runs resume cleanly. Adds a one-shot ab_nummer->timestamp
translation for existing shops transitioning from the legacy cursor.

Fixes silent data loss when more than 20 orders arrived between runs.
Issue OpenXE-org#262.
Captures lastImportTimestamp into a local variable before the pagination
loop so progress persistence inside the loop does not mutate the GET
filter. Without this, after=\$lastTs moves forward each iteration while
page advances too, causing 100 orders per extra page to be skipped.

Also fixes two smaller issues:
- resolveAbNummerToTimestamp() returns ts-1 so the strictly-after
  filter does not lose the transition order.
- explode(';', \$this->statusPending) is now PHP 8.1+ safe via (string)
  cast.

Follow-up to abe58aa, addresses code review findings on issue OpenXE-org#262.
Captures scope, fix parameters (MAX_PAGES_PER_RUN=5, 30-day first-run
fallback, UTC timestamps), implementation steps, integration test matrix
T1-T10, rollout and rollback strategy, risks and mitigations.

Companion doc to issue OpenXE-org#262 and the fix commits on this branch.
$this->app->DatabaseService is only lazy-bound in the web context. When
the shopimporter runs through the cron trigger the service is not
available, which breaks the timestamp persistence path. Falls back to
$this->app->DB with real_escape_string when DatabaseService is absent.

Discovered during the WC 8.9.3 + 10.7.0 integration test matrix on the
.143 test instance.
WCHttpClient::authenticate() had isQueryStringAuth() smuggled into its
SSL-gating during f12b09a. That changed the auth scheme for existing
HTTP-configured shops from OAuth 1.0a to basic-auth-over-query-string
(since query_string_auth=true is set at client construction site).
Restores the pre-f12b09a4 behaviour.

The CLI-context fallback for persistLastImportTimestamp from f12b09a
is kept.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The caller in shopimport.php uses $result[0] per iteration of a for loop
capped by ImportGetAuftraegeAnzahl() and maxmanuell. Returning 500
orders per call therefore silently dropped 499 of them while advancing
the server-side after-cursor past them. Restores the historical 1-order
contract; the after-filter still replaces the legacy 800-id include
hack, and per-order persist gives us resume-after-crash semantics with
at most one order lost per crash (consistent with pre-OpenXE-org#262 behaviour).

MAX_PAGES_PER_RUN and ORDERS_PER_PAGE constants are removed; the caller
loop (bounded by maxmanuell, default 100) now owns the batch size.

Follow-up to review on OpenXE-org#262.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
After the external review of OpenXE-org#262 highlighted that shopimport.php
expects $result[0] per RemoteGetAuftrag iteration, the internal
pagination loop was dropped. Plan now reflects: single-order per call,
caller loop bounded by maxmanuell, per-order progress persist.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… when nothing to import

shopimport.php:1308 gates on is_array($result), which happily accepts []
and then crashes trying to dereference $result[0]['id']. The pre-OpenXE-org#262
behaviour returned null on empty — restore that.

Spotted by review of 7af9edb.
The defensive getKonfig($data['shopid'] ?? null, $data) call inside
ImportGetAuftrag() is actively harmful: CatchRemoteCommand('data')
returns the getauftrag-payload, which does not carry a shopid (see
class.remote.php:194/241). The re-init therefore clears $this->shopid
and rebuilds the WCClient from empty preferences. RemoteCommand()
already initialises the importer with the real shop id before dispatch
(class.remote.php:2685), so the duplicate call is both redundant and
broken. Spotted by re-review of OpenXE-org#262.
shopimport.php calls RemoteGetAuftraegeAnzahl() before
RemoteGetAuftrag() in its main flow. If the stored cursor is still the
30-day fallback and all pending orders are older than 30 days, the
count returns 0 and ImportGetAuftrag() never runs, so the one-shot
ab_nummer -> timestamp migration never fires and the shop stays on the
fallback forever.

Extract the migration into a private helper and invoke it from both
count and fetch paths. Idempotent via lastImportTimestampIsFallback.
The after-filter is strictly-greater-than, so orders sharing an
identical date_created_gmt with the last processed order were silently
dropped. Move to a tuple cursor: persist both timestamp and order id,
query with after=<ts-1s> plus exclude=[last_id]. Orders with the same
GMT second now reach the caller in subsequent iterations without
duplicating the already-processed one.

Schema is additive (new felder.letzter_import_order_id key in
shopexport.einstellungen_json). Persistence helper becomes
persistLastImportCursor; the single-argument
persistLastImportTimestamp remains as a wrapper so the ab_nummer
migration path keeps working without a second rewrite.
…ration

Reflects re-review findings: tuple cursor (ts, id) for same-second order
resilience, migration helper called from both count and fetch paths,
scope list updated to match current single-order design.
…s offset

Bei identischem date_created_gmt mehrerer Orders hielt exclude nur die
zuletzt importierte ID. Nach zwei Orders im selben Bucket wurde die
erste wieder sichtbar und Count- wie Fetch-Pfad liefen in eine
Endlosschleife.

Cursor persistiert jetzt die komplette Liste aller IDs innerhalb des
aktuellen Sekunden-Buckets (felder.letzter_import_order_ids als JSON-
Array). Bei Bucket-Wechsel wird die Liste zurueckgesetzt; bei gleichem
Bucket wird die neue ID angehaengt.

Gleichzeitig wird die -1s-Korrektur am Query gated: nur wenn mindestens
eine exclude-ID bekannt ist, wird der after-Filter um 1 Sekunde nach
hinten verschoben. Dadurch entfaellt die Doppel-Subtraktion nach der
ab_nummer-Migration (resolveAbNummerToTimestamp schon -1s, Query war
nochmal -1s -> 2s zurueck in der Vergangenheit). Der Erstlauf nach
Migration liefert jetzt exakt die ab_nummer-Order als Startpunkt.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@Avatarsia Avatarsia force-pushed the refactor/woocommerce-composer-sdk branch 2 times, most recently from a096d34 to ded8846 Compare April 21, 2026 11:49
Avatarsia and others added 10 commits April 21, 2026 13:55
…ails

If resolveAbNummerToTimestamp() cannot find the referenced legacy order
(404, missing date_created_gmt, etc.) the migration previously left the
importer on the volatile 30-day fallback, which is recomputed on every
run as now()-30d. The cron cycle would then re-scan the same rolling
window forever, multiplying API load and caller-dedup activity.

On resolution failure we now explicitly persist the current fallback
timestamp so the fallback flag flips to false and the lower bound stays
stable across runs. Also emits a warning so the operator can spot the
stale ab_nummer.

Spotted by review of OpenXE-org#265.
Previously, stock sync ran two HTTP requests per article (SKU lookup +
update). For n articles this was 2n roundtrips; at 1000 articles that
is 2000 requests, easily tripping hoster rate limits and aborting the
sync partway through.

Switch to the official WC REST v3 batch endpoints:
- Collect all SKUs up front, resolve them in one or a few
  products?sku=<csv>&per_page=100 lookups (map sku -> id).
- Send stock updates in chunks of up to 100 items via
  POST products/batch.
- Variations go through POST products/{parent}/variations/batch,
  grouped per parent product.

Partial errors in a batch response are logged per SKU without aborting
the rest of the sync. At 1000 articles this reduces request count
from 2000 to roughly 15-20.

WCClient::post() accepts array data and JSON-encodes it directly --
no new postBatch() helper needed.

Closes OpenXE-org#263.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Companion doc to issue OpenXE-org#263 and the batch-refactor commit on this
branch.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… level

processBatchResponse()-Aufrufe sind jetzt in try/catch eingeschlossen —
ein einzelner WC-Batch mit HTTP 5xx/Timeout bricht den Sync nicht mehr
ab, nachfolgende Chunks werden trotzdem verarbeitet.

Der Erfolgs-Log in processBatchResponse() nutzt jetzt ->info() statt
->error() (war falscher Log-Level).

Spotted by review of b96079b.
ImportSendListLager() wraps the per-chunk batch POSTs in try/catch but
the upstream SKU-to-ID lookup was bare: a single failing lookup chunk
(timeout, 5xx, rate-limit) aborted the whole sync before the unaffected
remaining chunks could even be processed. That gave the batch refactor
a larger failure domain than the pre-OpenXE-org#266 per-item path it replaced.

Lookup exceptions are now logged at error level and the loop continues
with the next chunk. Items missing from the SKU map are already handled
downstream (logged as not-found, skipped from the update batches).

Spotted by review of OpenXE-org#266.
…ency

Adds automattic/woocommerce 3.1.1 via Composer. Vendor files included
per project convention (vendor/ is tracked). Resolves issue OpenXE-org#264.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…m client

The upstream automattic/woocommerce SDK lacks two accessors required by
the OpenXE business logic:
  - Client::getLastResponse() (upstream only exposes $client->http->getResponse())
  - Response::getHeader($name) with case-insensitive lookup

Adds Xentral\Components\WooCommerce\ClientWrapper which wraps the upstream
Client and exposes both methods. ResponseWrapper normalises all header keys
to lowercase at construction time. ssl_ignore is mapped to the upstream
verify_ssl option; logger is accepted but unused (no upstream hook).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Removes all inline SDK classes (WCClient, WCHttpClient, WCResponse,
WCRequest, WCOAuth, WCBasicAuth, WCOptions, WCHttpClientException)
from shopimporter_woocommerce.php — 1374 lines deleted.

Business logic now uses:
  - new ClientWrapper(...) instead of new WCClient(...)
  - use Xentral\Components\WooCommerce\ClientWrapper
  - use Automattic\WooCommerce\HttpClient\HttpClientException

All @throws and catch blocks updated to HttpClientException. Fixes OpenXE-org#264.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…diagnostics

Constructor accepted a logger but discarded it, which removed the
structured WooCommerce API error logs (4xx/5xx responses, failed
requests) that the inline SDK used to produce. That was a regression
for operational diagnostics.

ClientWrapper now:
- stores the PSR-3 logger passed in by the shopimporter,
- logs HTTP >= 400 responses as warning (with a truncated body excerpt),
- logs HttpClientException as error before rethrowing,
- keeps the same public interface, no callsite changes needed.

Spotted by review of e433b5a.
vendor/composer/installed.php referenced an outdated HEAD (84fdb97)
from when the composer require was first run, not the final branch
state. Regenerated via composer install so InstalledVersions::
getRootPackage() returns the correct revision and composer
dump-autoload is a no-op on a fresh clone.

Spotted by review of 1a61123.
Avatarsia added 3 commits April 21, 2026 14:00
Upstream vendored code (e.g. vendor/automattic/woocommerce/composer.json)
ships with formatting choices we do not control. Without this, git diff
--check reports their trailing whitespace as if it were a review issue
on our branches.

Spotted by review of 1a61123.
This file's root-package reference is a post-facto artefact: it records
whatever HEAD was when `composer install` ran, so any following commit
puts it out of sync with the new HEAD. This sync-commit lets installed.php
reference the last non-sync commit (f26fb82). Merging the branch will
regenerate it against the merge commit automatically.

There is no way to make it reference its own commit hash without a
self-referencing cycle; if this becomes a CI concern later,
vendor/composer/installed.php could be moved to .gitignore on the
understanding that composer install runs at deploy time.
Post-rebase regeneration of the Composer root reference so
InstalledVersions::getRootPackage() describes the current branch tip
rather than the pre-rebase commit.

Spotted by review of OpenXE-org#267.
@Avatarsia Avatarsia force-pushed the refactor/woocommerce-composer-sdk branch from ded8846 to 2040194 Compare April 21, 2026 12:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Refactor] WooCommerce-Client als Composer-Dependency statt Inline-Fork

1 participant